Skip to content

fs cases: fix compile error#2900

Closed
Zhangshoukui wants to merge 2 commits into
apache:masterfrom
Zhangshoukui:fs_
Closed

fs cases: fix compile error#2900
Zhangshoukui wants to merge 2 commits into
apache:masterfrom
Zhangshoukui:fs_

Conversation

@Zhangshoukui

@Zhangshoukui Zhangshoukui commented Dec 18, 2024

Copy link
Copy Markdown
Contributor

Summary

Fix compile error:

kernel/fs/cases/fs_stream_test.c:279:70: error: format ‘%zi’ expects argument of type ‘signed size_t’, but argument 4 has type ‘int’ [-Werror=format=]
  279 |           syslog(LOG_ERR, "len = %zi != return value from fwrite = %zi",
      |                                                                    ~~^
      |                                                                      |
      |                                                                      long int
      |                                                                    %i
  280 |                  len, ret);
      |                       ~~~
      |                       |
      |                       int
kernel/fs/cases/fs_stream_test.c:311:69: error: format ‘%zi’ expects argument of type ‘signed size_t’, but argument 4 has type ‘int’ [-Werror=format=]
  311 |           syslog(LOG_ERR, "len = %zi != return value from fread = %zi",
      |                                                                   ~~^
      |                                                                     |
      |                                                                     long int
      |                                                                   %i
  312 |                  len, ret);
      |                       ~~~
      |                       |
      |                       int

Impact

buld

Testing

pass ci

kernel/fs/cases/fs_stream_test.c:279:70: error: format ‘%zi’ expects argument of type ‘signed size_t’, but argument 4 has type ‘int’ [-Werror=format=]
  279 |           syslog(LOG_ERR, "len = %zi != return value from fwrite = %zi",
      |                                                                    ~~^
      |                                                                      |
      |                                                                      long int
      |                                                                    %i
  280 |                  len, ret);
      |                       ~~~
      |                       |
      |                       int
kernel/fs/cases/fs_stream_test.c:311:69: error: format ‘%zi’ expects argument of type ‘signed size_t’, but argument 4 has type ‘int’ [-Werror=format=]
  311 |           syslog(LOG_ERR, "len = %zi != return value from fread = %zi",
      |                                                                   ~~^
      |                                                                     |
      |                                                                     long int
      |                                                                   %i
  312 |                  len, ret);
      |                       ~~~
      |                       |
      |                       int

Signed-off-by: zhangshoukui <zhangshoukui@xiaomi.com>
@nuttxpr

nuttxpr commented Dec 18, 2024

Copy link
Copy Markdown

[Experimental Bot, please feedback here]

No, this PR does not meet the NuttX requirements. While it identifies a compile error, it provides no context about the change that caused the error or what was done to fix it. It also lacks critical information for reviewers.

Here's why it's insufficient and what's missing:

  • Summary: "compile error" is not a helpful summary. It needs to describe the reason for the change (fixing a compile error), what part of the code was changed (the format string in fs_stream_test.c), and how the change addresses the error (presumably changing %zi to %i or changing the argument type). There's no mention of a related NuttX issue.
  • Impact: Again, simply stating "compile error" isn't enough. While the immediate impact is a build failure, the PR needs to describe the root cause of the error. Was existing code broken by a recent change? Was new code introduced with incorrect types? This helps understand the potential ripple effects. All other impact categories are unaddressed.
  • Testing: Completely missing. The PR needs to indicate the build host and target used for testing, along with logs from before the change (showing the compile error) and after (demonstrating the fix). This validates that the change actually resolves the problem.

Example of a better summary:

Fixes a compile error in fs_stream_test.c caused by using the incorrect format specifier %zi for an int argument in syslog. The format specifier has been corrected to %i to match the argument type, resolving the build failure. No related NuttX issue found.

Example of a better impact assessment:

  • Is new feature added? Is existing feature changed?: No. Bug fix.
  • Impact on user: NO.
  • Impact on build: YES. The code now compiles correctly. Previously, it would fail with the reported error.
  • Impact on hardware: NO.
  • Impact on documentation: NO.
  • Impact on security: NO.
  • Impact on compatibility: NO.
  • Anything else to consider: NO.

Example of basic testing information:

  • Build Host(s): Linux, x86_64, GCC 12.2
  • Target(s): qemu-x86_64

(Logs before showing the error, logs after showing successful compilation)

@cederom cederom left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you @Zhangshoukui :-)

Looks like GH detected conflict that needs to be resolved before merge? :-)

@xiaoxiang781216

Copy link
Copy Markdown
Contributor

@Zhangshoukui please rebase the patch to fix the conflict.

@xiaoxiang781216

Copy link
Copy Markdown
Contributor

let's track this pr instead: #2905

@Zhangshoukui Zhangshoukui deleted the fs_ branch April 8, 2025 07:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants